Conversation
…ust icon fill colors to white
Refactor state reset logic and adjust icon fill colors
--------- Co-authored-by: Nguyen Hoang Ky <nhktmdzhg@gmail.com> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
fix: ensure buffer is committed before reset and simplify engine state cleanup clean up fix: reset engine state after committing string and remove unused header file hmm
Ensure buffer is committed before resetting engine state
🎨 Palette: Update Add button state in Keymap Editor * refactor: consolidate row lookup logic and remove redundant icon update calls in editors * refactor: implement upsert logic for keymap and macro table entries to ensure consistent state updates * refactor: switch backup file format from .lotusbak to .json --------- Co-authored-by: hthienloc <huynhloc.contact@gmail.com>
Workaround for Chromium suggestions
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements, including a version bump to 1.8.1, the addition of black icon variants for the Breeze theme, and various bug fixes related to preedit mode, Wayland compatibility, and key handling. It also adds security enhancements for socket connections by validating the UID of the connecting process. My feedback highlights thread-safety concerns with getpwuid() in lotus-utils.cpp, potential visibility issues with the new white icons in the light Breeze theme, and a recommendation to avoid hardcoding the fcitx5 executable path in lotus-server.cpp.
src/lotus-utils.cpp
Outdated
| std::string buildSocketPath(const char* base_path_suffix) { | ||
| const char* username_c = std::getenv("USER"); | ||
| std::string path; | ||
| struct passwd* pw = getpwuid(getuid()); |
There was a problem hiding this comment.
The function getpwuid() is not thread-safe. Since buildSocketPath() can be called from different threads (e.g., via connect_uinput_server()), this can lead to race conditions. Please use the reentrant version getpwuid_r() to ensure thread safety.
| </sodipodi:namedview> | ||
| <path | ||
| style="fill:#000000;stroke:#ffffff;stroke-width:0.4;stroke-linecap:square;stroke-opacity:0" | ||
| style="fill:#ffffff;stroke:#ffffff;stroke-width:0.4;stroke-linecap:square;stroke-opacity:0" |
There was a problem hiding this comment.
The fill color has been changed to #ffffff (white). For the light Breeze theme, icons should typically be dark to ensure visibility against a light panel background. A white icon will likely be invisible. Please consider reverting this to a dark color like #000000.
style="fill:#000000;stroke:#ffffff;stroke-width:0.4;stroke-linecap:square;stroke-opacity:0"
| fill="currentColor" | ||
| id="path1" | ||
| style="fill:#000000" /> | ||
| style="fill:#ffffff" /> |
There was a problem hiding this comment.
The fill color has been changed to #ffffff (white). For the light Breeze theme, icons should typically be dark to ensure visibility against a light panel background. A white icon will likely be invisible. Please consider reverting this to #000000.
style="fill:#000000" />
| </sodipodi:namedview> | ||
| <path | ||
| style="fill:#000000;stroke:#ffffff;stroke-width:0.4;stroke-linecap:square;stroke-opacity:0" | ||
| style="fill:#ffffff;stroke:#ffffff;stroke-width:0.4;stroke-linecap:square;stroke-opacity:0" |
There was a problem hiding this comment.
The fill color has been changed to #ffffff (white). For the light Breeze theme, icons should typically be dark to ensure visibility against a light panel background. A white icon will likely be invisible. Please consider reverting this to a dark color like #000000.
style="fill:#000000;stroke:#ffffff;stroke-width:0.4;stroke-linecap:square;stroke-opacity:0"
| ssize_t ret = readlink(path, exe_path, sizeof(exe_path) - 1); | ||
| if (ret != -1) { | ||
| exe_path[ret] = '\0'; // NOLINT | ||
| if (strcmp(exe_path, "/usr/bin/fcitx5") == 0) { |
There was a problem hiding this comment.
The path to the fcitx5 executable is hardcoded as /usr/bin/fcitx5. This may not be correct on all systems, for example if fcitx5 is installed in /usr/local/bin or another custom prefix. This could lead to authorization failures for legitimate clients.
Consider making this path configurable at build time, similar to how FCITX5_LOTUS_SETTINGS_PATH is handled.
4 participants
No description provided.